fix(code-review): make the documented "." catch-all actually match - #1

Summary

The README's catch-all example does not work. diff_paths are matched with startswith(), so "." matches only paths that literally begin with a dot:

$ jq --argjson changed_files '["README.md","Scarb.toml"]' -f matcher.jq cfg.json
{"include":[]}          # diff_paths: ["."], exclude_paths: ["packages/"]

A repo following the documented pattern gets no review at all on a PR that touches only README.md, package.json, Scarb.toml, codecov.yml — anything not under a dotted path. It fails silently: no error, no skipped job, the agent simply never enters the matrix.

It is also easy to miss in review, because the pathspec half of the same value is correct. git diff -- . ':!packages/' scopes exactly right, so on the runs where the agent does trigger (a .github/** change, say) it reviews the right files. Only the trigger decision is wrong.

Found while porting game-components to this workflow (Provable-Games/game-components#122), which had to enumerate every top-level path as a workaround.

The fix

• Normalize "." / "./" / "" to the empty prefix in the matcher. Every path starts with "", so a catch-all matches everything, and exclude_paths still carves out the app dirs.
• Map "" back to "." when building diff_paths_str, since "" is not a valid git pathspec but "." is and means the same thing. So [""] and ["."] now behave identically on both halves.
• Document that these are literal prefixes, not globs. "src/**" matches nothing today and gives no warning — worth stating outright.

Verification

No behavior change for existing callers. I extracted the matcher from the workflow before and after, and ran both against the two live configs — denshokan-sdk and game-components — across 10 changed-file sets each:

differences: 0

The catch-all now works. With diff_paths: ["."], exclude_paths: ["packages/"]:

Emitted pathspec is valid for both spellings: git diff -- . :!packages/.

Edge cases: an empty agents array returns {"include":[]} rather than erroring, and "src/**" still matches nothing — now called out in the README instead of silently surprising the next person.

npx yaml@2.8.1 valid passes on the workflow.

Consumer impact (all 6 live callers audited)

gh search code 'code-review.yml@v1' --owner Provable-Games finds six consumers. I ran the pre- and post-patch matchers against every one of their real configs across 12 changed-file sets:

Only entries that are exactly ".", "./" or "" change meaning, so a config without one is bit-identical by construction.

budokan and metagame_extensions are the two repos the bug is currently biting. Their general agent almost never runs today — a README.md, package.json, docs/** or LICENSE PR matches nothing at all:

budokan              ["README.md"]                     [] -> [general]
budokan              ["contracts/a.cairo","README.md"] [contracts] -> [contracts,general]
metagame_extensions  ["docs/x.md"]                     [] -> [general]

That is the fix landing, not a regression — those PRs are going unreviewed right now. But it is a live behavior change on two active repos, so it should be a conscious merge rather than a surprise.

It cannot turn a green build red. The only exit 1 paths in this workflow are a missing config file and a non-zero exit from the codex CLI itself; findings are never gated on severity. Both repos call this from a standalone pr-review.yml with no aggregate job depending on review, so nothing gates a merge on it either. The blast radius is more review jobs, more PR comments, and more spend on those two repos.

After merge

This is backward-compatible (0 diffs above), so per the README's versioning note the v1 tag should move forward to pick it up — callers pin @v1. I have not moved the tag.

Once it does, game-components can drop its enumerated path list back to the documented two-line catch-all.

🤖 Generated with Claude Code